-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
parse for minor release number #36
base: develop
Are you sure you want to change the base?
Conversation
Correctly handle x.x.0 input as the original major release
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, let's put this in and submit a pull request to https://github.com/wp-cli/scaffold-command and https://github.com/johnbillion/query-monitor/
bin/install-wp-tests.sh
Outdated
@@ -59,6 +64,22 @@ install_wp() { | |||
else | |||
if [ $WP_VERSION == 'latest' ]; then | |||
local ARCHIVE_NAME='latest' | |||
elif [[ $WP_VERSION =~ [0-9]+\.[0-9]+ ]]; then | |||
# https serves multiple offers, whereas http serves single. | |||
download https://api.wordpress.org/core/version-check/1.7/ /tmp/wp-latest.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix indentation here.
bin/install-wp-tests.sh
Outdated
LATEST_VERSION=$(grep -o '"version":"'$VERSION_ESCAPED'[^"]*' /tmp/wp-latest.json | sed 's/"version":"//' | head -1) | ||
fi | ||
if [[ -z "$LATEST_VERSION" ]]; then | ||
local ARCHIVE_NAME="wordpress-$WP_VERSION" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation.
bin/install-wp-tests.sh
Outdated
if [[ -z "$LATEST_VERSION" ]]; then | ||
local ARCHIVE_NAME="wordpress-$WP_VERSION" | ||
else | ||
ARCHIVE_NAME="wordpress-$LATEST_VERSION" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Awesome! This will be super helpful. 📦 ship it
if [[ $WP_VERSION =~ ^[0-9]+\.[0-9]+$ ]]; then | ||
WP_TESTS_TAG="branches/$WP_VERSION" | ||
elif [[ $WP_VERSION =~ [0-9]+\.[0-9]+\.[0-9]+ ]]; then | ||
if [[ $WP_VERSION =~ [0-9]+\.[0-9]+\.[0] ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code is duplicated below in install_wp()
. it looks like all this section should be doing is deciding whether the WP_TESTS_TAG
should use branches/
or tags/
.
I would change this code section to the following and put it right at the top. it'll eliminate the need to over-complicate the rest of the logic in the other two functions
# shorten `1.2.0` to `1.2`
if [[ $WP_VERSION =~ [0-9]+\.[0-9]+\.[0] ]]; then
$WP_VERSION = $WP_VERSION%??;
fi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since the WP_TESTS_TAG
variable is only used in the install_test_suite()
function, you could also move this section into there, as opposed to leaving it in the main body.
No description provided.